Skip to content

fix(security): add input validation to makeDockerExec#2987

Merged
la14-1 merged 2 commits intomainfrom
fix/issue-2985
Mar 25, 2026
Merged

fix(security): add input validation to makeDockerExec#2987
la14-1 merged 2 commits intomainfrom
fix/issue-2985

Conversation

@la14-1
Copy link
Member

@la14-1 la14-1 commented Mar 25, 2026

Why: makeDockerExec accepted any string without validation. Adding an explicit non-empty guard makes the security boundary clear and prevents silent misuse.

Changes

  • Added input validation to makeDockerExec in orchestrate.ts
  • Throws on empty command (explicit security boundary)
  • Patch version bump (0.26.1 → 0.26.2)

Fixes #2985

-- refactor/code-health

Adds non-empty guard to makeDockerExec to make the security boundary
explicit and prevent silent misuse with empty commands.

Fixes #2985

Agent: code-health
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Member

@louisgv louisgv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Review

Verdict: APPROVED
Commit: f63fbbb

Findings

No security issues found. The change adds proper input validation to makeDockerExec to prevent empty command strings from being passed to Docker exec.

Security Analysis:

  • Input validation: The new check prevents empty/null commands, which is a defensive programming practice
  • Command injection: Already protected by existing shellQuote() function which properly escapes single quotes and rejects null bytes
  • No new attack surface: The validation only adds a check, doesn't introduce new code paths
  • Version bump: Properly incremented from 0.26.1 to 0.26.2 (patch level)

Tests

  • Lint: PASS (bunx @biomejs/biome lint - 0 errors)
  • Unit tests: PASS (orchestrate-cov.test.ts - 22/22 pass)
  • Shell scripts: N/A (no .sh files modified)
  • macOS compat: N/A (TypeScript only)

Notes

  • The makeDockerExec function lacks dedicated unit test coverage for the new validation, but existing orchestration tests exercise the function indirectly
  • The change addresses issue #2985 properly by adding fail-fast validation

-- security/pr-reviewer

@la14-1 la14-1 merged commit 7194058 into main Mar 25, 2026
5 checks passed
@la14-1 la14-1 deleted the fix/issue-2985 branch March 25, 2026 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security: Command injection risk in Docker exec wrapper (makeDockerExec)

2 participants